Skip to content

clippy: make tests work in stage 1 #144027

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

RalfJung
Copy link
Member

This finally fixes #78717 :)

Similar to what Miri already does, the clippy test step needs to carefully consider which compiler is used to build clippy and which compiler is linked into clippy (and thus must be used to build the test dependencies). On top of that we have some extra complications that Miri avoided by using cargo-miri for building its test dependencies: we need cargo to use the right rustc and the right sysroot, but rust-lang/cargo#4423 makes this quite hard to do. See the long comment in src/tools/clippy/tests/compile-test.rs for details.

Some clippy tests tried to import rustc crates; that fundamentally requires a full bootstrap loop so it cannot work in stage 1. I had to kind of guess what those tests were doing so I don't know if my changes there make any sense.

Cc @flip1995 @Kobzol

@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2025

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-clippy Relevant to the Clippy team. labels Jul 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 16, 2025

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

// set `--sysroot`, so we need to use bootstrap's rustc wrapper. That wrapper
// however has some staging logic that is hurting us here, so to work around
// that we set both the "real" and "staging" rustc to TEST_RUSTC, including the
// associated library paths.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be possible to use a different approach here... somehow, aux builds are working for clippy tests. It seems to be using clippy itself as the compiler for that, and that's somehow okay? So, we could try to do the same here, instead of all this TEST_RUSTC stuff. (That would also be closer to what Miri does.)

But TBH I found something that works so I am not overly inclined to debug a different approach. ;)

@Kobzol
Copy link
Member

Kobzol commented Jul 17, 2025

FWIW, I have a WIP branch (there are so many stacked changes to bootstrap that I'm doing rn that it takes a while to unload) where I refactor the building of tools, so that the correct compiler should always be chosen, and we don't need to do this careful dance for each tool and bootstrap Step separately. Essentially we'd invert the compiler "flow" in the tool, so that we pass it the build compiler instead of figuring it out from the tool, and also hopefully we'll be able to get rid of the implicit rmeta/rlib copy into the build compiler's sysroot and make that explicit.

Anyway, I'm fine with this specific fix if it helps the stage 1 Clippy tests in the meantime (it seems like there are some other blockers in the tests themselves, which can be resolved in parallel), just wanted to note that hopefully soonish this will all be unified away so that we always do The Right Thing™.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-clippy Relevant to the Clippy team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

x.py test --stage 0 src/tools/clippy does not work
6 participants